-
Notifications
You must be signed in to change notification settings - Fork 55
fix(EAV-372): settings lost on studio update #1455
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: release52
Are you sure you want to change the base?
fix(EAV-372): settings lost on studio update #1455
Conversation
properties not present in apiStudio needed to be taken from the existing studio
... and add test coverage
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
… not generate invalid overrides, and correctly preserve existing overrides
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes an issue where updating Studio (Blueprint) Config via the HTTP API caused other Studio settings (like peripheral devices) to be lost. The fix preserves existing studio data when building updated studio objects.
- Refactored the
updateOverrides
function to properly preserve existing overrides and handle complex object structures - Modified the studio building logic to preserve existing studio properties before applying API updates
- Added comprehensive test coverage for the new override update behavior
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
packages/corelib/src/settings/objectWithOverrides.ts | Completely rewrote updateOverrides function with improved logic for preserving overrides and handling nested objects |
packages/corelib/src/settings/tests/objectWithOverrides.spec.ts | Added extensive test cases covering various override update scenarios |
meteor/server/api/rest/v1/typeConversion.ts | Refactored studio creation to preserve existing studio fields using spread operator |
meteor/server/api/rest/v1/tests/typeConversions.spec.ts | Added new test file with tests for studio field preservation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
value: rawValue, | ||
}) | ||
} | ||
if (typeof curValue === 'object' && typeof rawValue === 'object') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This condition doesn't handle null values properly since typeof null === 'object'
in JavaScript. This could lead to errors when trying to recursively process null values. Add null checks: if (typeof curValue === 'object' && curValue !== null && typeof rawValue === 'object' && rawValue !== null)
if (typeof curValue === 'object' && typeof rawValue === 'object') { | |
if ( | |
typeof curValue === 'object' && curValue !== null && | |
typeof rawValue === 'object' && rawValue !== null | |
) { |
Copilot uses AI. Check for mistakes.
About the Contributor
This pull request is posted on behalf of TV 2 Norge.
Type of Contribution
This is a:
Bug fix
Current Behavior
Updating Studio (Blueprint) Config using the HTTP API (updateStudioConfig operation) resulted in other Studio settings, e.g. peripheral devices to be lost.
New Behavior
Settings of the existing studio are preserved
Testing
Affected areas
This PR affects the HTTP API, specifically operations that update a Studio
Time Frame
Not urgent for us
Other Information
Status